Skip to content

Conversation

@ashishrp-aws
Copy link
Contributor

Problem

Setting up new Q Agentic Chat loop with tool use.

Solution

Initial Agentic Chat loop Setup with tool use


  • Treat all work as PUBLIC. Private feature/x branches will not be squash-merged at release time.
  • Your code changes must meet the guidelines in CONTRIBUTING.md.
  • License: I confirm that my contribution is made under the terms of the Apache 2.0 license.

@ashishrp-aws ashishrp-aws requested review from a team as code owners March 25, 2025 00:13
@github-actions
Copy link

  • This pull request modifies code in src/* but no tests were added/updated.
    • Confirm whether tests should be added or ensure the PR description explains why tests are not required.
  • This pull request implements a feat or fix, so it must include a changelog entry (unless the fix is for an unreleased feature). Review the changelog guidelines.
    • Note: beta or "experiment" features that have active users should announce fixes in the changelog.
    • If this is not a feature or fix, use an appropriate type from the title guidelines. For example, telemetry-only changes should use the telemetry type.

@ashishrp-aws ashishrp-aws reopened this Mar 25, 2025
@ashishrp-aws ashishrp-aws changed the base branch from master to feature/agentic-chat March 25, 2025 00:13
[key: string]: any
}

export function triggerPayloadToChatRequest(triggerPayload: TriggerPayload): { conversationState: ConversationState } {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we move the changes to a new function triggerPayloadToAgenticChatRequest

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need a new one? we can use the same function and add the API request blocks based on triggerPayload right?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, then should tools also be part of triggerPayload? Because it's not needed for mynah

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not needed, but i can make sure that Origin is present to exclude tools from the request. I'll do it in followup

const customizationArn: string | undefined = undefinedIfEmpty(triggerPayload.customization.arn)
const chatTriggerType = triggerPayload.trigger === ChatTriggerType.InlineChatMessage ? 'INLINE_CHAT' : 'MANUAL'

const tools: Tool[] = Object.entries(toolsJson as Record<string, ToolSpec>).map(([toolName, toolSpec]) => ({
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like this won't be easily scalable to other use cases like MCP where arbitrary tools can be added. Is there a better way to do this?

Copy link
Contributor Author

@ashishrp-aws ashishrp-aws Mar 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can have a number of static tools. I have moved this to constants. Agreed will check for a better way to handle tools.

let result: any
const toolResults: ToolResult[] = []
try {
switch (toolUse.name) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

did you comment things out here intentionally?

Copy link
Contributor Author

@ashishrp-aws ashishrp-aws Mar 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes....write tools aren't merged yet.

@laileni-aws laileni-aws requested a review from mr-lee March 25, 2025 18:05
export const tools: Tool[] = Object.entries(toolsJson).map(([, toolSpec]) => ({
toolSpecification: {
...toolSpec,
inputSchema: { json: toolSpec.inputSchema },
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: I think we don't need to do this anymore because we changed the tool index from index_schema to indexSchema

@ctlai95
Copy link
Contributor

ctlai95 commented Mar 25, 2025

Synced up offline. Please follow up with:

  • Format history (remove tools from user input message)
  • Fix history so that user input message is not the last message before the API call

@jpinkney-aws jpinkney-aws merged commit 2dbe485 into aws:feature/agentic-chat Mar 25, 2025
16 of 17 checks passed
@ashishrp-aws ashishrp-aws deleted the feature/agentic-chat branch March 25, 2025 20:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants